Fix instance() behaviour with pytz offsets#158
Merged
sdispater merged 1 commit intoNov 8, 2017
Conversation
`pytz` has a class of timezone called a `FixedOffset`. This class only stores an offset, not a zone name. When calling `instance()` on a datetime that was using one of these `FixedOffset` objects as a timezone, Pendulum would totally ignore the offset and use the local timezone instead. This would happen even if a `tz` argument was directly supplied to the `instance()` call. Additionally, the fallback to a fixed offset would always use the datetime's timezone, even it if was `None` and a `tz` argument was supplied to the `instance()` call. This commit makes 2 small changes: - The `zone` property of the "effective timezone" (the datetime's `tzinfo` with a fallback to the passed in `tz`) is only used if it is non-null. This fixes using `FixedOffset` objects as timezones. - The fallback to a fixed offset now uses the offset of the "effective timezone" instead of the datetime's offset. This correctly handles cases where the timezone on the datetime is `None` and a fallback is provided.
Collaborator
|
Nice catch! Thanks a lot for this PR :-) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
pytzhas a class of timezone called aFixedOffset. This class onlystores an offset, not a zone name.
When calling
instance()on a datetime that was using one of theseFixedOffsetobjects as a timezone, Pendulum would totally ignore theoffset and use the local timezone instead. This would happen even if a
tzargument was directly supplied to theinstance()call.Additionally, the fallback to a fixed offset would always use the
datetime's timezone, even it if was
Noneand atzargument wassupplied to the
instance()call.This commit makes 2 small changes:
zoneproperty of the "effective timezone" (the datetime'stzinfowith a fallback to the passed intz) is only used if it isnon-null. This fixes using
FixedOffsetobjects as timezones.timezone" instead of the datetime's offset. This correctly handles
cases where the timezone on the datetime is
Noneand a fallback isprovided.